240: Create new filters param on previous leaderboard endpoint#909
240: Create new filters param on previous leaderboard endpoint#909MalihaT111 wants to merge 1 commit into
Conversation
Available PR Commands
See: https://github.com/tahminator/codebloom/wiki/CI-Commands |
Ticket Validation FailedThe following issues were found with the attached ticket:
|
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
A[Leaderboard API Call] --> B{Filters parameter provided?};
B -- Yes --> C[Parse filters set];
B -- No --> D[Use individual boolean parameters];
C --> E[Build LeaderboardFilterOptions];
D --> E;
E --> F[Get leaderboard users];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Ticket Validation FailedThe following issues were found with the attached ticket:
|
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
Client --> LeaderboardController["LeaderboardController.java"]
LeaderboardController -- "Processes 'filters' param" --> LeaderboardFilterOptions["LeaderboardFilterOptions (built)"]
LeaderboardFilterOptions --> LeaderboardManager["LeaderboardManager.getLeaderboardUsers"]
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Ticket Validation FailedThe following issues were found with the attached ticket:
|
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
A[Client] --> B[Leaderboard Endpoint];
B -- New 'filters' param --> C{Filter Logic};
C --> D[LeaderboardManager];
D --> E[Filtered Data];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement, Tests Description
Diagram Walkthroughflowchart LR
A[LeaderboardController.getLeaderboardUsersById] --> B{Is 'filters' parameter null or empty?};
B -- Yes --> C[Use individual boolean parameters];
B -- No --> D[Parse filters Set for options];
C --> E[Build LeaderboardFilterOptions];
D --> E;
E --> F[Call leaderboardManager.getLeaderboardUsers];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
A[LeaderboardController.getLeaderboardUsersById] --> B{Is 'filters' parameter provided and not empty?};
B -- Yes --> C[Build LeaderboardFilterOptions from 'filters' Set];
B -- No/Empty --> D[Build LeaderboardFilterOptions from individual boolean parameters];
C --> E[Call leaderboardManager.getLeaderboardUsers];
D --> E;
|
| Relevant files | |||
|---|---|---|---|
| Api changes |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
02396ae to
97c87d6
Compare
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement Description
Diagram Walkthroughflowchart LR
A[LeaderboardController.getLeaderboardUsersById] --> B{Is 'filters' parameter provided and not empty?};
B -- Yes --> C[Derive filter options from 'filters' Set];
B -- No --> D[Derive filter options from individual boolean parameters];
C --> E[Build LeaderboardFilterOptions];
D --> E;
E --> F[Call leaderboardManager.getLeaderboardUsers];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement, Tests Description
Diagram Walkthroughflowchart LR
A[Client Request] --> B{LeaderboardController.getLeaderboardUsersById};
B -- new filters param --> C{Is 'filters' Set provided and not empty?};
C -- Yes --> D[Use 'filters' Set to build options];
C -- No --> E[Fall back to individual boolean params];
D --> F[Call leaderboardManager.getLeaderboardUsers];
E --> F;
F --> G[Return Leaderboard Data];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
/deploy |
1 similar comment
|
/deploy |
leaderboard endpoint and fixed leaderboard tests.
Title240: Create new filters param on previous leaderboard endpoint PR TypeEnhancement, Tests Description
Diagram Walkthroughflowchart LR
A[API Request] --> B{Filters parameter provided?};
B -- Yes --> C{Parse filters Set};
B -- No --> D{Use individual boolean params};
C --> E[Build LeaderboardFilterOptions];
D --> E;
E --> F[Get Leaderboard Users];
|
| Relevant files | |||
|---|---|---|---|
| Enhancement |
| ||
| Tests |
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
| final LeaderboardFilterOptions options = LeaderboardFilterOptions.builder() | ||
| .page(page) | ||
| .pageSize(parsedPageSize) | ||
| .query(query) | ||
| .patina(patina) | ||
| .hunter(hunter) | ||
| .nyu(nyu) | ||
| .baruch(baruch) | ||
| .rpi(rpi) | ||
| .gwc(gwc) | ||
| .sbu(sbu) | ||
| .ccny(ccny) | ||
| .columbia(columbia) | ||
| .cornell(cornell) | ||
| .bmcc(bmcc) | ||
| .mhcplusplus(mhcplusplus) | ||
| .patina(useFilters ? activeFilters.contains("patina") : patina) | ||
| .hunter(useFilters ? activeFilters.contains("hunter") : hunter) | ||
| .nyu(useFilters ? activeFilters.contains("nyu") : nyu) | ||
| .baruch(useFilters ? activeFilters.contains("baruch") : baruch) | ||
| .rpi(useFilters ? activeFilters.contains("rpi") : rpi) | ||
| .gwc(useFilters ? activeFilters.contains("gwc") : gwc) | ||
| .sbu(useFilters ? activeFilters.contains("sbu") : sbu) | ||
| .ccny(useFilters ? activeFilters.contains("ccny") : ccny) | ||
| .columbia(useFilters ? activeFilters.contains("columbia") : columbia) | ||
| .cornell(useFilters ? activeFilters.contains("cornell") : cornell) | ||
| .bmcc(useFilters ? activeFilters.contains("bmcc") : bmcc) | ||
| .mhcplusplus(useFilters ? activeFilters.contains("mhcplusplus") : mhcplusplus) | ||
| .build(); |
There was a problem hiding this comment.
Suggestion: The new conditional logic for setting each filter in the LeaderboardFilterOptions builder is highly repetitive. Encapsulating this logic in a private helper method would significantly improve readability and maintainability, especially if more filters are added or the logic changes. This aligns with the "Abstraction / Code Structure" guideline. [general, importance: 7]
| final LeaderboardFilterOptions options = LeaderboardFilterOptions.builder() | |
| .page(page) | |
| .pageSize(parsedPageSize) | |
| .query(query) | |
| .patina(patina) | |
| .hunter(hunter) | |
| .nyu(nyu) | |
| .baruch(baruch) | |
| .rpi(rpi) | |
| .gwc(gwc) | |
| .sbu(sbu) | |
| .ccny(ccny) | |
| .columbia(columbia) | |
| .cornell(cornell) | |
| .bmcc(bmcc) | |
| .mhcplusplus(mhcplusplus) | |
| .patina(useFilters ? activeFilters.contains("patina") : patina) | |
| .hunter(useFilters ? activeFilters.contains("hunter") : hunter) | |
| .nyu(useFilters ? activeFilters.contains("nyu") : nyu) | |
| .baruch(useFilters ? activeFilters.contains("baruch") : baruch) | |
| .rpi(useFilters ? activeFilters.contains("rpi") : rpi) | |
| .gwc(useFilters ? activeFilters.contains("gwc") : gwc) | |
| .sbu(useFilters ? activeFilters.contains("sbu") : sbu) | |
| .ccny(useFilters ? activeFilters.contains("ccny") : ccny) | |
| .columbia(useFilters ? activeFilters.contains("columbia") : columbia) | |
| .cornell(useFilters ? activeFilters.contains("cornell") : cornell) | |
| .bmcc(useFilters ? activeFilters.contains("bmcc") : bmcc) | |
| .mhcplusplus(useFilters ? activeFilters.contains("mhcplusplus") : mhcplusplus) | |
| .build(); | |
| final LeaderboardFilterOptions options = LeaderboardFilterOptions.builder() | |
| .page(page) | |
| .pageSize(parsedPageSize) | |
| .query(query) | |
| .patina(getFilterValue(useFilters, activeFilters, "patina", patina)) | |
| .hunter(getFilterValue(useFilters, activeFilters, "hunter", hunter)) | |
| .nyu(getFilterValue(useFilters, activeFilters, "nyu", nyu)) | |
| .baruch(getFilterValue(useFilters, activeFilters, "baruch", baruch)) | |
| .rpi(getFilterValue(useFilters, activeFilters, "rpi", rpi)) | |
| .gwc(getFilterValue(useFilters, activeFilters, "gwc", gwc)) | |
| .sbu(getFilterValue(useFilters, activeFilters, "sbu", sbu)) | |
| .ccny(getFilterValue(useFilters, activeFilters, "ccny", ccny)) | |
| .columbia(getFilterValue(useFilters, activeFilters, "columbia", columbia)) | |
| .cornell(getFilterValue(useFilters, activeFilters, "cornell", cornell)) | |
| .bmcc(getFilterValue(useFilters, activeFilters, "bmcc", bmcc)) | |
| .mhcplusplus(getFilterValue(useFilters, activeFilters, "mhcplusplus", mhcplusplus)) | |
| .build(); | |
| // Add this private helper method to the LeaderboardController class: | |
| private boolean getFilterValue(boolean useFilters, Set<String> activeFilters, String filterName, boolean defaultValue) { | |
| return useFilters ? activeFilters.contains(filterName) : defaultValue; | |
| } |
240
Description of changes
Added filters parameter (a set of strings) that store the items that belong in the leaderboard url. Added an if else statement for when filters is empty or null. When it's empty or null it goes back to previous url.
Checklist before review
Screenshots
Dev
Current:

New:

Staging